fix: post-merge bug hunt — six fixes + review doc#32
Conversation
Bug-hunt review on master (see BUG_REVIEW.md). Fixes: - send_chunks: bench cap no longer zeroes the final partial chunk - Param routing: activation auto-detect works when --instrument is set - TSV: use mzML column layout (no spurious Title column) - GF SpecE: honor is_protein_n_term for Met-cleaved peptides - TSV: write IsotopeError from psm.isotope_offset - CLI: reject inverted charge/isotope error ranges Adds regression tests for bench mode and inverted charge range. Documents five additional open findings for follow-up. Co-authored-by: Cursor <cursoragent@cursor.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Correct mod-aware pepSeq dedup to use rank_score with deterministic survivor selection, add isotope-range validation test, and optimize the dedup hot path with Arc-cached integer keys and FxHashMap charge queues. Co-authored-by: Cursor <cursoragent@cursor.com>
Align README and DOCS with post-B2/B5 behavior, correct PIN column count, document inverted range validation, and replace removed known-divergences.md references with DOCS §8d. Co-authored-by: Cursor <cursoragent@cursor.com>
Resolves conflicts after PR #33 (MassCalibrator port — opt-in --precursor-cal) merged into dev. Two minor conflicts: - crates/msgf-rust/src/bin/msgf-rust.rs: PR-A moved `spectrum_ext` and `ms_level_u32` declarations earlier in run(); the local block at the former position was redundant. Took PR-A's side (empty block). - crates/msgf-rust/tests/cli_smoke.rs: kept PR-A's two new tests (cli_accepts_isotope_error_min_negative_one, cli_accepts_precursor_cal_off) alongside the bug-hunt branch's existing CLI smoke tests.
1 participant
Summary
Systematic bug review of the merged Rust port (
master@ PR #31). AddsBUG_REVIEW.mddocumenting 11 findings; fixes the six highest-confidence issues in this PR.Fixes (B1–B6)
--max-spectrabench cap dropped the final partial chunk to zero--instrument≠low-resTitlecolumn)IsotopeErrorcolumn always 0Open follow-ups (documented, not in this PR)
Test plan
cargo test --release -p msgf-rust --test cli_smoke→ 10 passed (incl. new bench-mode + charge-range tests)cargo test --release --workspacewith 7 CI-known skips → 0 new failuresMade with Cursor